Add unit tests for osism/api.py HTTP and WebSocket endpoints - #2485
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| "stderr,expected_status", | ||
| [ | ||
| ("Could not match supplied host pattern: node-1", 404), | ||
| ("Unable to parse /inventory/hosts.yml", 404), |
There was a problem hiding this comment.
Both hostvars handlers (osism/api.py:829 and :892) fold Unable to parse into the same 404 branch as Could not match, so a corrupt or unreadable inventory is reported to the client as Host '<host>' not found in inventory — a server/config fault surfaced as a missing resource, which sends an operator hunting for a host that may well exist. This row pins both the wrong status and the false detail string.
Rather than dropping the row, please fix it properly in a separate commit in this PR: split the guard so Could not match stays 404 while Unable to parse returns a 500-class status, then update this row to expect that. Keep the Could not match -> 404 and other error -> 500 rows unchanged.
|
|
||
| assert response.status_code == 200 | ||
| body = response.json() | ||
| assert body["hosts_searched"] == 2 |
There was a problem hiding this comment.
hosts_searched is len(hosts_to_search) computed before the loop (osism/api.py:1095), so it counts hosts the limit break at :1098 never visits — it's a candidate count, not hosts actually searched. In a separate commit, initialize it to 0 and increment once per host entered in the loop, then update the assertions to the corrected counts.
This particular case stays == 2 (an attempted-but-failed host is still searched); the overcount to fix is the limit-short-circuit path, so add a hosts_searched assertion to test_search_limit_stops_early, which currently only checks run.call_count.
| assert run.call_count == 1 # only the --list call, no per-host lookups | ||
|
|
||
|
|
||
| def test_search_limit_stops_early(client, mocker): |
There was a problem hiding this comment.
limit has no lower bound (osism/api.py:1021, limit: int = 100), so limit=0 or a negative value passes validation and reaches the accumulation loop. In a separate commit, constrain it (e.g. limit: int = Query(default=100, gt=0)) and add a test asserting a 422 for non-positive values. Nothing here asserts the current behavior, so no existing row needs to change.
github-project-automation
Bot
moved this from Ready for review
to In review
in Human Board
Exercise all routes of the OSISM API server through fastapi.testclient.TestClient in a new tests/unit/test_api_endpoints.py: - Health and telemetry sink endpoints: ok responses for JSON lists and objects, 500 with endpoint-specific detail for malformed bodies - Baremetal endpoints: node list mapping into BaremetalNode, empty list, NetBox info for single and multiple nodes (including 422 for a missing node_names field), ports and parameters, error paths with the node name/UUID in the detail - notifications_baremetal: handler dispatch via BaremetalEvents, handler errors, non-UUID message_id rejection, and the real default handler for unknown event types - sonic_ztp_complete: 503 without NetBox, provision_state update and device.save(), 404 for unknown identifiers, save() failures - webhook: 503 without NetBox, parsed WebhookNetboxData handed to process_netbox_webhook, processing errors, 422 for missing fields - WebSocket /v1/events/openstack: connect/disconnect lifecycle, set_filters acknowledgment with full and partial filters, ignored invalid JSON and non-filter actions, and connection survival after update_filters errors (with a stubbed websocket manager) - Inventory endpoints: hosts listing with --limit handling, hostvars with secret masking and sorted variables, ansible-inventory error mapping (404/500/504), facts from the Redis cache, and the search endpoint including regex/source validation, host filtering, the facts source, the result limit, masking before matching, and per-host failure tolerance The module-level helpers and Pydantic models are covered separately in tests/unit/test_api_helpers.py. Closes #2360 Assisted-by: Claude:claude-fable-5 Signed-off-by: Christian Berendt <berendt@osism.tech>
A corrupt or unreadable inventory makes ansible-inventory print "Unable to parse" to stderr. Both hostvars endpoints folded this into the same 404 "Host '<host>' not found in inventory" branch as "Could not match", so a server/config fault was reported to the client as a missing resource, sending operators hunting for a host that may well exist. Only "Could not match" now maps to 404; any other stderr, including "Unable to parse", falls through to the generic 500 response. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christian Berendt <berendt@osism.tech>
hosts_searched was set to len(hosts_to_search) before the loop, so it counted candidate hosts that the limit break never visited. It now starts at 0 and increments once per host actually entered in the loop, making it a count of hosts searched rather than hosts selected. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christian Berendt <berendt@osism.tech>
limit had no lower bound, so limit=0 or a negative value passed validation and reached the accumulation loop. It is now declared as Query(default=100, gt=0), so non-positive values are rejected with a 422 before any inventory work is done. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christian Berendt <berendt@osism.tech>
027e160 to
ace2f71
Compare
3 participants
Adds
tests/unit/test_api_endpoints.py, exercising all HTTP endpoints and the WebSocket endpoint ofosism/api.pythroughfastapi.testclient.TestClient, as specified in #2360:/,/v1,/v1/events, and both sink endpoints (JSON list, JSON object, malformed body → 500 with endpoint-specific detail)BaremetalNode, empty list, NetBox info for single and multiple nodes (including 422 for a missingnode_namesfield), ports, parameters, and error paths asserting the node name/UUID in the detailBaremetalEvents, handler errors → 500, non-UUIDmessage_id→ 422, and the real default handler for unknown event types → 204provision_stateupdate plusdevice.save(), 404 for unknown identifiers, save failures → 500WebhookNetboxDatahanded toprocess_netbox_webhook, processing errors → 500, missing required field → 422/v1/events/openstack: connect/disconnect lifecycle,set_filtersacknowledgment with full and partial filters, ignored invalid JSON and non-filter actions, connection survival afterupdate_filterserrors (stubbed websocket manager, so no broadcaster task is started)--limithandling, hostvars with secret masking and sorted variables, ansible-inventory error mapping (404/500/504), facts from the Redis cache with the separator-less cache key, and the search endpoint (regex/source validation, host filtering, facts source, result limit with early loop exit, masking before matching, per-host failure tolerance, query echo, and bothget_inventory_pathcall variants)httpxwas already added to thePipfiledev packages by #2359, so no dependency changes are needed. The module-level helpers and Pydantic models are covered separately bytests/unit/test_api_helpers.py.Closes #2360
🤖 Generated with Claude Code
https://claude.ai/code/session_01P5LudkHsw8ZXnyFmRHQDRE